Skip to content

Conversation

camsteffen
Copy link
Contributor

@camsteffen camsteffen commented Dec 1, 2021

changelog: upgrade [map_flatten] to complexity

Resolves #7999

@rust-highfive
Copy link

r? @giraffate

(rust-highfive has picked a reviewer for you, use r? to override)

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties label Dec 1, 2021
@flip1995
Copy link
Member

flip1995 commented Dec 1, 2021

I remember that @mikerite had strong and reasonable opinions on the map+another-method-call lints. So I would like to hear their opinion on moving this lint to style.

@ghost
Copy link

ghost commented Dec 2, 2021

Looks good to me but the one thing I would consider is if this lint belongs in style or complexity.

Note that classifying the lint as complexity discussed in #5846 and they went with pedantic.

I would be surprised if anyone objects to moving it to warn-by-default:

I ran it on around 1000 top crates and I got 11 warnings. The worse was plotters with 4.

Warnings - 11
criterion-0.3.5 - 3
gif-0.11.3 - 1
plotters-0.3.1 - 4
racer-2.1.48 - 1
raw-cpuid-10.2.0 - 1
rustyline-9.0.0 - 1

---> criterion-0.3.5/src/plot/plotters_backend/summary.rs:34:57                                                                                                                                       
warning: called `map(..).flatten()` on an `Iterator`                                                                                                                                                  
  --> src/plot/plotters_backend/summary.rs:34:57
   |
34 |         plotters::data::fitting_range(series_data.iter().map(|(_, xs, _)| xs.iter()).flatten());
   |                                                         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try using `flat_map` instead: `.flat_map(|(_, xs, _)| xs.iter())`
   |
   = note: requested on the command line with `-W clippy::map-flatten`
   = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#map_flatten


---> criterion-0.3.5/src/plot/plotters_backend/summary.rs:36:57                                                                                                                                       
warning: called `map(..).flatten()` on an `Iterator`                                                                                                                                                  
  --> src/plot/plotters_backend/summary.rs:36:57
   |
36 |         plotters::data::fitting_range(series_data.iter().map(|(_, _, ys)| ys.iter()).flatten());
   |                                                         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try using `flat_map` instead: `.flat_map(|(_, _, ys)| ys.iter())`
   |
   = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#map_flatten


---> criterion-0.3.5/src/plot/plotters_backend/summary.rs:200:50                                                                                                                                      
warning: called `map(..).flatten()` on an `Iterator`                                                                                                                                                  
   --> src/plot/plotters_backend/summary.rs:200:50
    |
200 |         plotters::data::fitting_range(kdes.iter().map(|(_, xs, _)| xs.iter()).flatten());
    |                                                  ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try using `flat_map` instead: `.flat_map(|(_, xs, _)| xs.iter())`
    |
    = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#map_flatten


---> gif-0.11.3/src/common.rs:242:40                                                                                                                                                                  
warning: called `map(..).flatten()` on an `Iterator`                                                                                                                                                  
   --> src/common.rs:242:40
    |
242 |         let palette = colors_vec.iter().map(|&(r, g, b, _a)| vec![r, g, b]).flatten().collect();
    |                                        ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try using `flat_map` instead: `.flat_map(|&(r, g, b, _a)| vec![r, g, b])`
    |
    = note: requested on the command line with `-W clippy::map-flatten`
    = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#map_flatten

---> plotters-0.3.1/src/coord/ranged1d/combinators/group_by.rs:94:32                                                                                                                                  
warning: called `map(..).flatten()` on an `Iterator`                                                                                                                                                  
  --> src/coord/ranged1d/combinators/group_by.rs:94:32
   |
94 |               return outter_ticks
   |  ________________________________^
95 | |                 .map(|base| inner_ticks.iter().map(move |&ofs| base * self.1 + ofs))
96 | |                 .flatten()
   | |__________________________^ help: try using `flat_map` instead: `.flat_map(|base| inner_ticks.iter().map(move |&ofs| base * self.1 + ofs))`
   |
   = note: requested on the command line with `-W clippy::map-flatten`
   = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#map_flatten


---> plotters-0.3.1/src/coord/ranged1d/combinators/nested.rs:115:29                                                                                                                                   
warning: called `map(..).flatten()` on an `Iterator`                                                                                                                                                  
   --> src/coord/ranged1d/combinators/nested.rs:115:29
    |
115 |                   .enumerate()
    |  _____________________________^
116 | |                 .map(|(idx, val)| {
117 | |                     std::iter::once(NestedValue::Category(val)).chain(
118 | |                         self.secondary[idx]
...   |
125 | |                 })
126 | |                 .flatten()
    | |__________________________^
    |
    = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#map_flatten
help: try using `flat_map` instead
    |
115 ~                 .enumerate().flat_map(|(idx, val)| {
116 +                     std::iter::once(NestedValue::Category(val)).chain(
117 +                         self.secondary[idx]
118 +                             .key_points(secondary_size)
119 +                             .into_iter()
120 +                             .map(move |v| {
  ...


---> plotters-0.3.1/src/drawing/area.rs:60:17                                                                                                                                                         
warning: called `map(..).flatten()` on an `Iterator`                                                                                                                                                  
  --> src/drawing/area.rs:60:17
   |
60 |           (0..row)
   |  _________________^
61 | |             .map(move |x| repeat(x).zip(0..col))
62 | |             .flatten()
   | |______________________^ help: try using `flat_map` instead: `.flat_map(move |x| repeat(x).zip(0..col))`
   |
   = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#map_flatten


---> plotters-0.3.1/src/drawing/area.rs:96:25                                                                                                                                                         
warning: called `map(..).flatten()` on an `Iterator`                                                                                                                                                  
   --> src/drawing/area.rs:96:25
    |
96  |               .into_iter()
    |  _________________________^
97  | |             .map(move |(y0, y1)| {
98  | |                 xsegs
99  | |                     .clone()
...   |
102 | |             })
103 | |             .flatten()
    | |______________________^
    |
    = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#map_flatten
help: try using `flat_map` instead
    |
96  ~             .into_iter().flat_map(move |(y0, y1)| {
97  +                 xsegs
98  +                     .clone()
99  +                     .into_iter()
100 +                     .map(move |(x0, x1)| Self { x0, y0, x1, y1 })
101 +             })
    |

---> racer-2.1.48/src/racer/nameres.rs:1945:25                                                                                                                                                        
warning: called `map(..).flatten()` on an `Iterator`                                                                                                                                                  
    --> src/racer/nameres.rs:1945:25
     |
1945 |               .into_iter()
     |  _________________________^
1946 | |             .map(|m| {
1947 | |                 search_for_trait_items(m, &following_seg.name, search_type, true, true, session)
1948 | |             })
1949 | |             .flatten()
     | |______________________^
     |
     = note: requested on the command line with `-W clippy::map-flatten`
     = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#map_flatten
help: try using `flat_map` instead
     |
1945 ~             .into_iter().flat_map(|m| {
1946 +                 search_for_trait_items(m, &following_seg.name, search_type, true, true, session)
1947 +             })
     |


---> raw-cpuid-10.2.0/src/lib.rs:731:46                                                                                                                                                               
warning: called `map(..).flatten()` on an `Option`                                                                                                                                                    
   --> src/lib.rs:731:46
    |
731 |               .filter(|fi| fi.has_hypervisor())
    |  ______________________________________________^
732 | |             .map(|_| {
733 | |                 let res = self.read.cpuid1(EAX_HYPERVISOR_INFO);
734 | |                 if res.eax > 0 {
...   |
742 | |             })
743 | |             .flatten()
    | |______________________^
    |
    = note: requested on the command line with `-W clippy::map-flatten`
    = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#map_flatten
help: try using `and_then` instead
    |
731 ~             .filter(|fi| fi.has_hypervisor()).and_then(|_| {
732 +                 let res = self.read.cpuid1(EAX_HYPERVISOR_INFO);
733 +                 if res.eax > 0 {
734 +                     Some(HypervisorInfo {
735 +                         read: self.read,
736 +                         res,
  ...


---> rustyline-9.0.0/src/edit.rs:285:27                                                                                                                                                               
warning: called `map(..).flatten()` on an `Option`                                                                                                                                                    
   --> src/edit.rs:285:27
    |
285 |         self.hint.as_ref().map(|hint| hint.completion()).flatten()
    |                           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try using `and_then` instead: `.and_then(|hint| hint.completion())`
    |
    = note: requested on the command line with `-W clippy::map-flatten`
    = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#map_flatten

It's allowed only once on Noisy Clippy.

@camsteffen
Copy link
Contributor Author

Thanks for the extra info! complexity makes sense to me.

@camsteffen camsteffen changed the title Upgrade map_flatten to style Upgrade map_flatten to complexity Dec 3, 2021
@giraffate
Copy link
Contributor

@bors r+

Thanks!

@bors
Copy link
Contributor

bors commented Dec 4, 2021

📌 Commit de9de4f has been approved by giraffate

@bors
Copy link
Contributor

bors commented Dec 4, 2021

⌛ Testing commit de9de4f with merge 9eabec9...

@bors
Copy link
Contributor

bors commented Dec 4, 2021

☀️ Test successful - checks-action_dev_test, checks-action_remark_test, checks-action_test
Approved by: giraffate
Pushing 9eabec9 to master...

@bors bors merged commit 9eabec9 into rust-lang:master Dec 4, 2021
@camsteffen camsteffen deleted the map-flatten-style branch December 4, 2021 13:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-review Status: Awaiting review from the assignee but also interested parties
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Categorize map_flatten as a style lint instead of pedantic
5 participants